Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make retrieval of parameters more robust in Frame python bindings #422

Merged
merged 1 commit into from
May 30, 2023

Conversation

tmadlener
Copy link
Collaborator

BEGINRELEASENOTES

  • Fix small bug in Frame python bindings where set but empty parameters could crash podio-dump when trying to access a non-existent element

ENDRELEASENOTES

@tmadlener tmadlener requested a review from jmcarcell May 30, 2023 09:40
@jmcarcell
Copy link
Member

In which cases can you have set but empty parameters?

@tmadlener
Copy link
Collaborator Author

That came up during the LCIO -> EDM4hep conversion. Not sure why, but there are cases where there are parameter keys without any non-empty value associated to it.

Copy link
Member

@jmcarcell jmcarcell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without being able to reproduce it looks good, the bug is quite obvious when pointed out. As long as the returned parameters are in a list (even an empty list) this will work fine

EDIT: I was able to reproduce independently and can confirm this fixes the issue

@tmadlener tmadlener merged commit 4a767cf into AIDASoft:master May 30, 2023
@tmadlener tmadlener deleted the fix-params-frame branch May 30, 2023 14:25
tmadlener added a commit that referenced this pull request Jul 13, 2023
* Make retrieval of parameters more robust in Frame python bindings (#422)

* Make ROOTLegacyReader handle older versions correctly

* Fix reading of v00-16 files and adapt tests to expected contents
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants